Theme detection and update improvements #237
Conversation
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
|
I really like this approach! Not having to deal with custom matplotlib style sheet paths makes life a lot simpler for us, and also gives the user only one place to customise the theme instead of having to do it for both napari and napari-matplotlib. I need to do some manual testing, but in the meantime would you be happy chaging either the code or tests to make the tests pass? If you're confused by the image tests (they are confusing!) just let me know and I can take a look at updating them. I automatically fixed formatting issues, so there's now an extra commit on your branch. If you want to do taht yourself locally you can install and run pre-commit: https://pre-commit.com/ |
|
Hi, I'm happy you like my suggestions. I can take a look at the tests but I'm rather short on time at the moment so this could take some time. |
|
I've updated the image tests, and cleaned out the old styling code that isn't needed any more. I still need to debug some of the new images, as the text colour doesn't look right. Manual testing indicates this is looking good otherwise though 👍 |
| # callback to update when napari theme changed | ||
| # TODO: this isn't working completely (see issue #140) | ||
| # most of our styling respects the theme change but not all |
There was a problem hiding this comment.
Yes it fixes the axelabel text colour issue, but is kind of a work around for the background issue.
All the text on the canvas (except in he legend) is now linked to the napari text colour which should always be high contrast to the background colour.
The background is transparent, which is not optimal in all cases, but for what ever reason I was not able to implement it in a way that the background is updated correctly.
I guess there might be an underlying bug not directly connected to napari-matplotlib but either napari or and this is my guess the way matplotlib updates the canvas.
|
Looks like all the tests work now. Is there still something I should check? |
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
|
Nope, I think this all looks good now 👍 . I'll wait for the tests to pass again then merge. Will try and get this in a new release in the next few weeks or so, thanks a lot for the PR! |
|
Happy I could help ^^ |
Hi,
as mentioned in the issue #236 I did a first draft of a possible implementation of dynamic styling based on the
napariviewer theme.This rework completely omits the concept of static style sheets for the different
naparitheme and instead translates the current viewer theme into amatplotlibcompatible style dictionary.This happens upon widget initiation and the style dictionary is stored in the attribute:
napari_theme_style_sheet.The style dictionary can be directly used for a
motplotlib.style.contect.Additionally, I fixed the viewer theme callback and the style change upon theme change works now properly.
I encountered some strange during the rework, if I set the
facecolorof the figure or axes to something other than transparent (none) the color will stay. Seems like thefacecolors are not updated by redraws. I'm feel this is amatplotlibbug, but I could be wrong, maybe I just used the wrong functions for redrawing the figure and axis faces.EDIT:
I can reproduce this behaviour with pure
matplotlib, if I explicitly set the facecolor and redraw it changes, but if I use a style context it stays the same.If you find the implementation suitable you or I would need to give the test an update as well.